Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update arg_parser.py to fix --port argument type #6

Merged
merged 3 commits into from
Apr 23, 2015

Conversation

lfyzjck
Copy link

@lfyzjck lfyzjck commented Apr 23, 2015

--port argument defualt type is str, this will cause a consul register error.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.92% when pulling bde7eea on lfyzjck:master into 0f3d1e1 on ClearcodeHQ:master.

@@ -143,6 +143,7 @@ def get_parser():

parser.add_argument(
"--port",
type=int, default=0,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default 0 ? I think it's not even possible to use such port.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps there is no default value here, argument --port still equals to 0 even leave it empty.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 to 1023 are "well known ports" but there is no document that says 0 cannot be used and it has no official protocol assigned for neither TCP nor UDP. 0 is sometimes used (unofficialy) for specifying system allocated ports

@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.92% when pulling 94aa37b on lfyzjck:master into 0f3d1e1 on ClearcodeHQ:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.92% when pulling d7abdf0 on lfyzjck:master into 0f3d1e1 on ClearcodeHQ:master.

@swistakm
Copy link
Collaborator

build failed due to #4

@swistakm
Copy link
Collaborator

someone could retry the build manually. I don't have such access rights anymore.

@lfyzjck
Copy link
Author

lfyzjck commented Apr 23, 2015

can't reappear this test failure in local environment.

@pwilczynskiclearcode
Copy link

someone could retry the build manually. I don't have such access rights anymore.

I granted you access to ianitor back again.

swistakm added a commit that referenced this pull request Apr 23, 2015
update arg_parser.py to fix `--port` argument type
@swistakm swistakm merged commit ca1f27d into ClearcodeHQ:master Apr 23, 2015
@swistakm
Copy link
Collaborator

Merged and released on PyPI as 0.0.3.

@pwilczynskiclearcode thanks for access grant!
@lfyzjck thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants